Skip to content

ENH: Add generate-lcov-html flag#258

Closed
ganesh-k13 wants to merge 1 commit intoscientific-python:mainfrom
ganesh-k13:bld_coverage
Closed

ENH: Add generate-lcov-html flag#258
ganesh-k13 wants to merge 1 commit intoscientific-python:mainfrom
ganesh-k13:bld_coverage

Conversation

@ganesh-k13
Copy link
Contributor

@ganesh-k13 ganesh-k13 commented Dec 5, 2024

Changes

  • Added generate-lcov-html flag in test

Example

~/os/devpy/example_pkg (main) » spin test --gcov --coverage
Invoking `build` prior to running tests:                                                                                                                                            
Removing previous GCOV .gcda files...                                                                                                                                               
$ meson setup build --prefix=/usr -Db_coverage=true
The Meson build system        
Version: 0.64.0                            
.
.
.
.
.
---------- coverage: platform linux, python 3.10.12-final-0 ----------
Name                                            Stmts   Miss  Cover
-------------------------------------------------------------------
example_pkg/__init__.py                             3      0   100%
example_pkg/conftest.py                             0      0   100%
example_pkg/submodule/__init__.py                   0      0   100%
example_pkg/submodule/tests/__init__.py             0      0   100%
example_pkg/submodule/tests/test_submodule.py       2      0   100%
example_pkg/tests/__init__.py                       0      0   100%
example_pkg/tests/test_core.py                      4      0   100%
-------------------------------------------------------------------
TOTAL                                               9      0   100%
Coverage HTML written to dir /home/ganesh/os/devpy/example_pkg/build/coverage/
~/os/devpy/example_pkg (main) » spin test --generate-lcov-html
Deleting old HTML coverage report...
Generating HTML coverage report...
$ ninja coverage-html -C build
Coverage report generated successfully and written to /home/ganesh/os/devpy/example_pkg/build/meson-logs/coveragereport/index.html

coverage_report

Other error handling

» spin build  
» spin test --generate-lcov-html
Error: GCOV files missing... Cannot generate coverage reports. Coverage reports can be generated by `spin test --coverage --gcov`

Related

ToDo

  • Add UT

@ganesh-k13 ganesh-k13 marked this pull request as draft December 5, 2024 13:46
@rgommers rgommers mentioned this pull request Dec 7, 2024
Merged
18 tasks
@czgdp1807
Copy link
Contributor

czgdp1807 commented Dec 7, 2024

Adding one point here. I think its ninja which calls lcov when we pass --generate-lcov-html to spin (trying on this PR) ninja doesn't call lcov with --no-external flag. So the end result will include headers from dependencies as well. See details in scipy/scipy#21919 (comment) and scipy/scipy#21919 (comment). If there's a way to pass options to lcov via ninja CLI then we should pass --no-external flag.

stefanv
stefanv reviewed Dec 9, 2024
View reviewed changes
spin/cmds/meson.py
from .util import get_commands, get_config
from .util import run as _run

LCOV_OUTPUT_FILE = Path("build", "meson-logs", "coveragereport")
Copy link
Member

@stefanv stefanv Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will depend on the build path the user specifies, so has to be set inside the build fn.

spin/cmds/meson.py
default="html",
help=f"Format of the gcov report. Can be one of {', '.join(e.value for e in GcovReportFormat)}.",
)
@click.option(
Copy link
Member

@stefanv stefanv Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding a new flag, what do you think of adding an html-lcov option to --gcov-format?

Copy link
Contributor Author

@ganesh-k13 ganesh-k13 Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it makes sense to club it. It would need a bit of rework I have used the enum to denote the coverage format directly. Let me see how we can make it neater and more generic.

@stefanv stefanv added the type: Enhancement New feature or request label Dec 9, 2024
@stefanv
Copy link
Member

stefanv commented Dec 9, 2024

Thanks @ganesh-k13!

@ganesh-k13
Copy link
Contributor Author

ganesh-k13 commented Dec 12, 2024

Adding one point here. I think its ninja which calls lcov when we pass --generate-lcov-html to spin (trying on this PR) ninja doesn't call lcov with --no-external flag. So the end result will include headers from dependencies as well. See details in scipy/scipy#21919 (comment) and scipy/scipy#21919 (comment). If there's a way to pass options to lcov via ninja CLI then we should pass --no-external flag.

@czgdp1807 , I searched quite a bit but I don't see any way to pass args to lcov via ninja. That being said, I'm not seeing external libraries listed for NumPy (example: numpy/numpy#24992). I'm trying to see how that's happening.

@czgdp1807
Copy link
Contributor

czgdp1807 commented Dec 13, 2024

Hi. So you can check my uploaded coverage reports for SciPy - https://github.com/czgdp1807/scipy-lcov-genhtml-coverage-output.

On main branch of above repository,

Screenshot 2024-12-13 at 1 26 48 PM

On no_external branch of above repository (passed --no-external flag here)

Screenshot 2024-12-13 at 1 27 56 PM Screenshot 2024-12-13 at 1 28 04 PM

So, you can see, with --no-external results for only SciPy files are shown and without it, dependencies are also covered (noisy).

I tried to search for an environment variable through which we can pass CLI options to lcov. But there isn't such any. Google search results gave LCOV_HOME but that doesn't seem to be correct.

@ganesh-k13
Copy link
Contributor Author

ganesh-k13 commented Dec 27, 2024 •
edited
Loading

Bit of an oversight from me, but seems like --gcov-format html for Python and --gcov for C/CPP gives the exact same result as --generate-lcov-html in this PR. So I assume I can close this now?

@czgdp1807 , regarding the external library coverage being included, can you see if you get the same results for NumPy in your machine. If not, I assume it's something to do with the way SciPy is built that is different that we need to deep dive on.

@czgdp1807
Copy link
Contributor

czgdp1807 commented Jan 3, 2025

I verified. https://github.com/czgdp1807/scipy-lcov-genhtml-coverage-output contains the latest reports. I think we are good. spin test --gcov --coverage --gcov-format=html generates both Python coverage and C/C++/Fortran coverage. It calls "lcov" only for those files which are within SciPy. Go ahead and close this PR for now.

@ganesh-k13 ganesh-k13 closed this Jan 9, 2025
@ganesh-k13 ganesh-k13 mentioned this pull request Jan 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stefanv stefanv stefanv left review comments

Assignees

No one assigned

Labels

type: Enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ganesh-k13 @czgdp1807 @stefanv